Hey - in the future, can you ping me when you make changes to ./script/tool_runner.sh analyze that might effect how other repos should drive it? Thanks!

https://dart-review.googlesource.com/c/sdk/+/207981

Seems like we should have a comment or ci check to try to avoid this in the future. I didn't even know the dart bots ran flutter plugins tests, makes sense though.

Or we could make a script that wraps up the analyze invocation so we just have to change it in one repo.

Hey - in the future, can you ping me when you make changes to ./script/tool_runner.sh analyze that might effect how other repos should drive it? Thanks!

Sorry, I completely forgot you'd set that up in the Dart CI :(

Now that we have the config file it may be safer to have that test use flutter_plugin_tools directly rather than using the wrapper script.

I think either moving off of using the wrapper script and putting a comment in the yaml file, or making a trivial dedicated wrapper per @gaaclarke's suggestion, are the best long-term options, since the goal now is to minimize, or even eliminate, the current shell script

I do expect some (infrequent) breakages due to driving this from an upstream repo. But whatever we can do to make this reasonably stable - with most of the knowledge of how to analyze the repo in the repo itself, and only a small entry-point that something off-repo would need to depend on - would be great.

For reference, this is where we drive it from (from our analyzer pre-submit bot): https://github.com/dart-lang/sdk/blob/master/tools/bots/flutter/analyze_flutter_plugins.sh. The less that script has to know about the internals of the flutter/plugins repo to more stable it'll be.

A wrapper script is safer since it's a single point of failure, it's just not ideal that we would have to run that one command differently forever.

If we put the details of the call in the Dart repo script it will break if we change how analyze should be called (fairly unlikely) or we change that file (e.g., eliminate it once everything is off the legacy options). We could put heavily commented unit test that uses exactly the args you put in that script though. That would have the advantage of covering the custom SDK argument, which you are the only client of (so someone could change it remove later without realizing it would break you, currently).

My preference would be for the latter if you're okay with occasional update as long as we have a process (the test) for making sure we don't forgot to tell you like I did this time.

I suspect that longer term we'd want the --custom-analysis arg to go away? Perhaps have it default to a config file in the repo if no --custom-analysis is passed in, so that bare uses of ./script/tool_runner.sh analyze will just work?

I suspect that longer term we'd want the --custom-analysis arg to go away?

The long term goal is to not have anything in the repo that has its own analysis options: flutter/flutter#76229

Perhaps have it default to a config file in the repo if no --custom-analysis is passed in, so that bare uses of ./script/tool_runner.sh analyze will just work?

I considered this, including making the test exclusion configs a big config file that was broken down by test type and platform that was auto-included. My preference is to make things that we are doing that we don't want to be doing (like using legacy options, or not running tests for some plugins) explicit though.